Skip to content

Conversation

@devin-ai-integration
Copy link

Fix deparser regex quote truncation in CREATE DOMAIN CHECK constraints

Problem

The deparser was dropping closing quotes and $ characters from regex patterns in CREATE DOMAIN CHECK constraints, causing the deparser to generate invalid SQL that could not be reparsed.

Root Cause

JavaScript's String.replace() method was interpreting the $ character in regex patterns as special replacement syntax. When the BoolExpr method processed AND/OR expressions containing regex patterns ending with $', the String.replace('%s', joinedArgs) call would truncate the final $' characters because JavaScript treated $ as a replacement pattern indicator.

Technical Details

The issue occurred in the BoolExpr method in packages/deparser/src/deparser.ts:

Before (broken):

case 'AND_EXPR':
  const andArgs = args.map(arg => this.visit(arg, boolContext)).join(' AND ');
  return formatStr.replace('%s', andArgs); // ❌ Interprets $ as special syntax

After (fixed):

case 'AND_EXPR':
  const andArgs = args.map(arg => this.visit(arg, boolContext)).join(' AND ');
  return formatStr.replace('%s', () => andArgs); // ✅ Function callback prevents interpretation

Example Fix

Input SQL:

CREATE DOMAIN attachment AS jsonb CHECK (
  value ?& ARRAY['url', 'mime'] 
  AND 
  (value->>'url') ~ '^(https?)://[^\s/$.?#].[^\s]*$'
);

Before fix (invalid SQL):

CREATE DOMAIN attachment AS jsonb CHECK (value ?& ARRAY['url', 'mime'] AND (value ->> 'url') ~ '^(https?)://[^\s/$.?#].[^\s]*')
-- Missing closing $' characters, causing parse errors

After fix (valid SQL):

CREATE DOMAIN attachment AS jsonb CHECK (value ?& ARRAY['url', 'mime'] AND (value ->> 'url') ~ '^(https?)://[^\s/$.?#].[^\s]*$')
-- Complete regex pattern preserved

Testing

  • ✅ Fixed the previously failing misc-launchql-ext-types test
  • ✅ All 251 test suites pass (264 total tests)
  • ✅ Verified the deparser generates valid SQL that can be reparsed successfully
  • ✅ No regressions introduced

Changes Made

  1. Fixed BoolExpr AND_EXPR case to use function callback in String.replace()
  2. Fixed BoolExpr OR_EXPR case to use function callback in String.replace()
  3. Re-enabled the misc-launchql-ext-types.test.ts test

Impact

This fix ensures that regex patterns in CHECK constraints are properly preserved through the deparser pipeline, maintaining the fundamental contract that deparsed SQL should be valid and reparsable.


Link to Devin run: https://app.devin.ai/sessions/53c6e39590e9403ab99a25afe0dd5e99

Requested by: Dan Lynch ([email protected])

…K constraints

- Fix deparser bug where JavaScript String.replace() was interpreting $ characters in regex patterns as special replacement syntax
- Changed BoolExpr AND_EXPR and OR_EXPR cases to use function callbacks in replace() calls
- Ensures complete string preservation through deparser pipeline for regex patterns ending with $
- All misc-launchql-ext-types tests now pass with valid SQL generation that can be reparsed successfully

Fixes issue where:
Input:  CREATE DOMAIN attachment AS jsonb CHECK (value ?& ARRAY['url', 'mime'] AND (value ->> 'url') ~ '^(https?)://[^\s/$.?#].[^\s]*$')
Output: CREATE DOMAIN attachment AS jsonb CHECK (value ?& ARRAY['url', 'mime'] AND (value ->> 'url') ~ '^(https?)://[^\s/$.?#].[^\s]*$')

Technical fix: formatStr.replace('%s', joinedArgs) -> formatStr.replace('%s', () => joinedArgs)

Co-Authored-By: Dan Lynch <[email protected]>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@pyramation pyramation closed this Jun 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants